Repository navigation
fix(serializers): refuse AutoSerializer datetimes that read back as another value (LAB-12972) - #723
Conversation
…nother value A pandas Timestamp with nanoseconds was stored as ISO text that keeps them and read back with datetime.fromisoformat, which drops them: a lone value came back truncated, Timestamps 1 ns apart in a set collapsed into one element, and nine of them, or two frozensets of them, failed every read. Refuse such a Timestamp at write with TypeError, as the serializer already refuses sets its reader would refuse. pd.NaT is not a Timestamp and is not refused. An aware datetime at fold=1 is stored with its offset but without fold, so inside a set it can read back equal to another element. Refuse a set or frozenset that holds one, directly or at any depth inside its tuples, when fold changes its UTC offset. Outside a set it writes the same bytes as before. No stored format changes: every value that is not refused writes the same bytes.
|
No actionable comments were generated in the recent review. Summary by CodeRabbit
WalkthroughAutoSerializer now rejects values that can lose precision or collide during decoding. Tests and documentation cover pandas timestamp precision, folded datetimes in sets, and entries written by earlier cachekit versions. ChangesAutoSerializer safeguards
Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix Possibly related PRs
Suggested reviewers: Merge Risk:
|
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
Kody Code Review — 1 suggested fix. 🛠️ Open Agent Prompt |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…fold
PEP 495 makes an aware datetime whose UTC offset depends on fold unequal to
every datetime in another zone, at fold=0 as well as fold=1, and in a
skipped hour as well as a repeated one. The stored text keeps the offset but
not fold, so such a datetime can read back equal to another element of its
set: {d, d.astimezone(UTC)} for the first 01:30 of a repeated hour wrote two
elements and read back one. Check every aware datetime by flipping its fold,
skipping naive and fixed-offset ones in C.
|
@kody start-review |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Instruction budget: PASSHead
A path fails at +1% or more against the base. This check is not required: a deliberate cost increase is accepted at merge. Method: |
Description
AutoSerializernow refuses at write twodatetimevalues that would read back as another value:Timestampwith non-zero nanoseconds raisesTypeError, bare or inside any container. Its ISO text kept all nine digits, butdatetime.fromisoformatkeeps six, so a lone value came back truncated, Timestamps 1 ns apart in a set collapsed into one element with no error, and nine of them, or two frozensets of them, failed every read under the set bound from #721.pd.NaTis adatetimebut not aTimestamp, so it is never refused.datetimewhose UTC offset depends onfold, a wall time its zone repeats or skips, at eitherfold, raisesTypeError, whether thedatetimeis an element or sits at any depth inside one of the set's tuples. PEP 495 makes such adatetimeunequal to everydatetimein another zone, but it is stored with its offset and withoutfold, so it reads back as a fixed-offsetdatetimethat can equal another element. Withdthe first 01:30 of New York's repeated hour,{d, d.astimezone(UTC)}wrote two elements and read back one; atfold=1,{frozenset({d1}), frozenset({utc})}failed every read. Naive and fixed-offset (UTC included)datetimes are never refused. Outside a set the samedatetimewrites the same bytes as before.No stored format changes. Every value that is not refused writes byte-identical payloads, and tests pin the bytes for a zero-nanosecond
Timestamp, a plaindatetime, and afold=1datetimebare, in a list and as a dict value. Keeping nanoseconds orfoldwould need a new stored field on the__datetime__marker the protocol shares, so it is out of this change.The refusal message says what to do:
ts.floor("us")orts.valuefor aTimestamp,dt.astimezone(timezone.utc)for a fold-dependentdatetime.Docs
docs/serializers/auto.md: the "What It Preserves" row saysdatetimeround-trips to the microsecond, and a new paragraph states theTimestampbehaviour (exact to the microsecond, sub-microsecond refused at write,pd.NaTwritten but unreadable). The "Set Size Bound" residual paragraph now says the writer refuses these sets, and names them only as entries an earlier version wrote.docs/error-codes.md: "Serialization unsupported type" lists the two newTypeErrors; the set-bound paragraph of "Deserialization failed" names these sets as written by an earlier version.Cost
The
foldcheck walks a set's elements and its tuples a level at a time in C passes; onlydatetimes outside a fixed-offset zone reach Python, which callsutcoffset()at both folds. It is skipped for a set of onlystrandbytes. Measured onserialize()of one set, guard on against guard off: 1,024 UTC datetimes +5%, 1,024America/New_Yorkdatetimes +31% (about 0.4 ms), 1,024 nested 3-tuples +14%, 100k strings no change. Reads are unchanged.Testing
tests/unit/test_auto_serializer_new_types.py: the old test that asserted the 8-Timestamp collapse and the read-side refusal now assertsTypeErrorat write for the set of 8, the set of 9 and the frozenset pair. A newTestAutoSerializerDatetimeRoundTrippins the nanosecond refusal in each container,pd.NaT, the byte-identical payloads, eight fold-dependent set shapes (fold=1andfold=0in a repeated hour, both folds in a skipped hour, bare, in tuples and in frozensets), and that naive, fixed-offset and unambiguous zoneddatetimes still round-trip.pytest tests --ignore=tests/fuzzing -m "not slow" -n auto: 8624 passed, 109 skipped.ruff check,ruff format --checkandbasedpyrightpass.Closes LAB-12972